Skip to content

Refreshing SQL ManagedDatabase SDK.#7808

Merged
isra-fel merged 6 commits intoAzure:masterfrom
misosic-msft:dev
Oct 22, 2019
Merged

Refreshing SQL ManagedDatabase SDK.#7808
isra-fel merged 6 commits intoAzure:masterfrom
misosic-msft:dev

Conversation

@misosic-msft
Copy link
Copy Markdown
Contributor

@misosic-msft misosic-msft commented Sep 30, 2019

Adding RestoreDetails API and modifying CompleteRestore.

markcowl
markcowl previously requested changes Oct 2, 2019
View reviewed changes
Comment thread ...oft.Azure.Management.SqlManagement/src/Generated/IManagedDatabaseRestoreDetailsOperations.cs
@jaredmoo
Copy link
Copy Markdown
Contributor

jaredmoo commented Oct 3, 2019

Adding E2E test?

@markcowl
Copy link
Copy Markdown
Member

markcowl commented Oct 4, 2019

@misosic-msft Please respond to @jaredmoo comment

@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 4, 2019

Adding E2E test?

I will add E2E test in the next commit.

@markcowl
Copy link
Copy Markdown
Member

markcowl commented Oct 7, 2019

  • looks good from my perspective, other than the testing comment from Jared. ny reason we need to merge this now instead of waiting for the tests in the next commit?

@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 7, 2019

  • looks good from my perspective, other than the testing comment from Jared. ny reason we need to merge this now instead of waiting for the tests in the next commit?

No need to merge before I add tests. Thanks!

@isra-fel isra-fel added the Mgmt This issue is related to a management package. label Oct 8, 2019
@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 15, 2019

@misosic-msft ping on this

@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 18, 2019

@misosic-msft ping on this

I've noticed a change needed before new API can be used from clients. ARM manifest needs to be updated. Either we can leave this PR open and waiting for other changes or we can close it.

@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 20, 2019

I've added the test for new API. All working as expected. This can be merged now.

@Azure Azure deleted a comment from azure-pipelines Bot Oct 21, 2019
@Azure Azure deleted a comment from azure-pipelines Bot Oct 21, 2019
@isra-fel isra-fel closed this Oct 21, 2019
@isra-fel isra-fel reopened this Oct 21, 2019
@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 21, 2019

/azp run net - mgmt - ci

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Oct 21, 2019

Azure Pipelines successfully started running 1 pipeline(s).

@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 21, 2019 •
edited
Loading

Sorry I accidently triggered the running of more CI tasks than it needs... but you only need to make sure net - mgmt - ci and its sub-jobs are passing. Thanks @misosic-msft

@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 21, 2019

I've fixed the failing checks now. Thanks @isra-fel

jaredmoo
jaredmoo reviewed Oct 21, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jaredmoo jaredmoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update version number in AssemblyInfo.cs and .csproj. Also update release notes in .csproj. Compare against the most recent release at https://www.nuget.org/packages/Microsoft.Azure.Management.Sql .

@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 22, 2019

jaredmoo is right. And given you were changing the API versions you use, please also update the tags in AzSDK.RP.props

isra-fel
isra-fel requested changes Oct 22, 2019
View reviewed changes
Comment thread sdk/eng/mgmt/mgmtmetadata/sql_resource-manager.txt Outdated
@misosic-msft
Copy link
Copy Markdown
Contributor Author

misosic-msft commented Oct 22, 2019

jaredmoo is right. And given you were changing the API versions you use, please also update the tags in AzSDK.RP.props

It's auto generated and contains 2018 version.

@misosic-msft misosic-msft reopened this Oct 22, 2019
@misosic-msft misosic-msft requested a review from isra-fel October 22, 2019 08:45
@isra-fel isra-fel merged commit d3a4cff into Azure:master Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isra-fel isra-fel isra-fel approved these changes

@markcowl markcowl markcowl left review comments

@erich-wang erich-wang Awaiting requested review from erich-wang

@AlexGhiondea AlexGhiondea Awaiting requested review from AlexGhiondea

+1 more reviewer

@jaredmoo jaredmoo jaredmoo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@misosic-msft misosic-msft

Labels

Mgmt This issue is related to a management package.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@misosic-msft @jaredmoo @markcowl @isra-fel